Skip to content

Spatialhash regional spherical improvements#2744

Open
fluidnumericsJoe wants to merge 3 commits into
mainfrom
spatialhash-regional-spherical-improvements
Open

Spatialhash regional spherical improvements#2744
fluidnumericsJoe wants to merge 3 commits into
mainfrom
spatialhash-regional-spherical-improvements

Conversation

@fluidnumericsJoe

Copy link
Copy Markdown
Contributor

Description

This PR changes the spatialhash initialization for spherical mesh types. Previously the bounding box was set to the unit cube for spherical mesh types. However, spherical mesh types might be desirable for regional simulations (e.g. for improved accuracy in barycentric coordinates, dealing with regions close to the poles, or for leveraging built in unit conversions in Parcels velocity field interpolators).

The main problem with the unit cube for regional configurations is that the hash grid resolution is often far too coarse for a regional domain. This results in a high number of mesh cells being aligned with a single morton key. Calling spatialhash.query() then results in an abnormally high number of particle in cell checks.

This is remedied here by choosing the extents of the hash grid as the bounding cube (x,y,z) of the regional domain.

Checklist

  • Tests added
  • This PR targets the correct branch (main for normal development, v3-support for v3 support)

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.
    • Describe how you used it (e.g., by pasting your prompt): Claude Code was used to identify and implement documentation changes.

When the mesh type is spherical, we now use the actual x,y,z bounding
box rather than the unit cube. For regional runs that leverage the
spherical barycentric coordinates and built in unit conversion in the
velocity field interpolators, this reduces the hit count per hash cell
and can improve the search time in the spatialhash.query()

Note that quantize_coordinates now clips in float space before casting to uint32.
This matters now that spherical queries can fall outside the hash-grid bounds
(previously impossible with the unit cube). A negative normalized value cast to
uint32 wraps to a huge number, which would have mapped below-range queries to the
top bin (with morton code 2^32) erroneously.

For spherical grids, points outside the regional box now quantize to edge bins
rather than interior bins. In this case, they either fail the exact Morton-key
match or are rejected by the point-in-cell check; either case results in the
same outcome as before (GRID_SEARCH_ERROR)
Tests for both points inside the domain and explicitly outside the domain
to confirm correct behavior for out of bounds PIC checks
@fluidnumericsJoe

Copy link
Copy Markdown
Contributor Author

I still need to test this out on the example you shared @erikvansebille, but wanted to get this in and make sure that existing tests all pass

@fluidnumericsJoe

Copy link
Copy Markdown
Contributor Author

looks like there's a few tests to clean up here.

@erikvansebille

Copy link
Copy Markdown
Member

Thanks for this PR, @fluidnumericsJoe! Seems like there are some segmentation errors in the tests. Do you want to try fix them; or can I help?

@fluidnumericsJoe

Copy link
Copy Markdown
Contributor Author

Thanks for this PR, @fluidnumericsJoe! Seems like there are some segmentation errors in the tests. Do you want to try fix them; or can I help?

I'll get to it today

@fluidnumericsJoe

Copy link
Copy Markdown
Contributor Author

So, I've narrowed down what's going on here. To recap what we primarily changed here - we've changed the spatialhash initialization to use a more narrow bounding box, based on the actual x,y,z extents for spherical mesh types. effectively, for regional domains this makes the hash grid size smaller. This happens because the number of hash grid cells is fixed, but we're now using a smaller domain extent for the hash grid.

The upshot of this change is that we have fewer faces in the "parent" mesh that we need to check, when a particle is found in a given hash cell. This reduces the number of calls to the particle in-cell checks.

The issue here is in the construction of the hash table that maps a hash cell index (the morton code) to a list of faces. During construction we have to use the bounding box of a face in the parent grid to find the range of hash cells that overlap. This information is used to pre-allocate a lookup table that maps from parent grid face to hash grid cells; the hash table is contructed as the inverse of that table. If the hash cells are significantly smaller than the parent model face sizes, we end up with a fairly large memory footprint (high number of hash cells per face).

I'm looking at tidying up that initializer here to reduce the memory footprint..

@fluidnumericsJoe

Copy link
Copy Markdown
Contributor Author
hashgrid_overview This figure shows the antimeridian spanning grid that the CI is failing on with OOM errors. The grid locations are plotted in cartesian (x,y,z) space after mapping the latitude, longitude through a spherical coordinate transformation. A box shows a region that we'll zoom in on in the next figure hashgrid_zoom This zoomed in region shows the hash grid (light gray lines) and the parent grid edges (blue lines). The bouding cube for the cell shaded light blue has 55x59x46 hash cells. Summing the number of hash cells per face across all faces (to get the size of the work arrays for constructing the hash table), each work array is roughly 193 million entries (too big).

In addition to tidying up in the initializer, I'll probably look at putting in a lever to manipulate the hash cell size - with the morton cells, this is controlled by the bit width . A higher bitwidth gives more hash cells per direction. At the moment, we default to 1024.

Another thing we could possibly do is be a bit smarter about tossing out hash cells that are clearly outside of elements (bounding box overlaps are kind of wasteful). In both plane views in the zoomed in plot, there's a large number of hash cells that are completely outside the element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants